Skip to content

Comments

Refactor connect-omi.py for improved device selection and user intera…#294

Closed
AnkushMalaker wants to merge 3 commits intodevfrom
feat/improved-laptop-connection
Closed

Refactor connect-omi.py for improved device selection and user intera…#294
AnkushMalaker wants to merge 3 commits intodevfrom
feat/improved-laptop-connection

Conversation

@AnkushMalaker
Copy link
Collaborator

@AnkushMalaker AnkushMalaker commented Feb 7, 2026

…ction

  • Replaced references to the chronicle Bluetooth library with friend_lite for device management.
  • Removed the list_devices function and implemented a new prompt_user_to_pick_device function to enhance user interaction when selecting OMI/Neo devices.
  • Updated the find_and_set_omi_mac function to utilize the new device selection method, improving the overall flow of device connection.
  • Added a new scan_devices.py script for quick scanning of neo/neosapien devices, enhancing usability.
  • Updated README.md to reflect new usage instructions and prerequisites for connecting to OMI devices over Bluetooth.
  • Enhanced start.sh to ensure proper environment variable setup for macOS users.

Summary by CodeRabbit

Release Notes

  • New Features

    • Button event capture and tracking during audio sessions
    • Interactive device discovery and selection with persistent preferences
    • New friend-lite-sdk Python package for OMI device connectivity
    • Transcription support via Deepgram and Wyoming backends
  • Documentation

    • Added SDK documentation and usage guides

…ction

- Replaced references to the chronicle Bluetooth library with friend_lite for device management.
- Removed the list_devices function and implemented a new prompt_user_to_pick_device function to enhance user interaction when selecting OMI/Neo devices.
- Updated the find_and_set_omi_mac function to utilize the new device selection method, improving the overall flow of device connection.
- Added a new scan_devices.py script for quick scanning of neo/neosapien devices, enhancing usability.
- Updated README.md to reflect new usage instructions and prerequisites for connecting to OMI devices over Bluetooth.
- Enhanced start.sh to ensure proper environment variable setup for macOS users.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 7, 2026

📝 Walkthrough

Walkthrough

This pull request introduces a button event tracking feature to the backend with markers storage and persistence, adds a comprehensive friend-lite-sdk library for BLE connectivity and transcription, and refactors local OMI BT tooling to use the new SDK.

Changes

Cohort / File(s) Summary
Backend Button Events & Markers
backends/advanced/src/advanced_omi_backend/client.py, websocket_controller.py, models/conversation.py, plugins/base.py, plugins/router.py, workers/conversation_jobs.py
Adds button event handling pipeline: ClientState now maintains markers list; websocket_controller processes button events, persists markers to Redis, and attaches them to conversations; MemoryVersion model adds markers field; plugin router dispatches button.event events; conversation_jobs retrieves markers from Redis and attaches to conversations.
friend-lite-sdk Library
extras/friend-lite-sdk/{LICENSE,NOTICE,README.md,pyproject.toml}, friend_lite/{__init__.py,bluetooth.py,button.py,decoder.py,discover_characteristics.py,transcribe.py,uuids.py}
New SDK module providing BLE device management (OmiConnection, device discovery), button state parsing, Opus audio decoding, UUID constants, and dual-backend transcription (Deepgram and Wyoming) with queue-driven audio handling.
Local OMI BT Refactoring
extras/local-omi-bt/{README.md,connect-omi.py,requirements.txt,scan_devices.py,send_to_adv.py,start.sh}
Refactors legacy BLE flow to use OmiConnection from friend-lite-sdk; adds interactive device selection with persistence, button event handling, BLE characteristic discovery script, and macOS Opus library path handling.

Sequence Diagram(s)

sequenceDiagram
    participant Device as OMI Device
    participant WS as WebSocket<br/>Controller
    participant ClientState as Client<br/>State
    participant Redis as Redis
    participant Router as Plugin<br/>Router
    participant Plugin as Plugin
    participant Conversation as Conversation<br/>Jobs

    Device->>WS: button-event message
    activate WS
    WS->>WS: _handle_button_event()
    WS->>WS: create marker dict<br/>(type, state, timestamp, etc.)
    WS->>ClientState: add_marker(marker)
    activate ClientState
    ClientState->>ClientState: append to markers list
    deactivate ClientState
    WS->>Router: dispatch button.event
    activate Router
    Router->>Plugin: on_button_event(context)
    activate Plugin
    Plugin-->>Router: PluginResult
    deactivate Plugin
    deactivate Router
    WS->>Redis: persist markers as JSON<br/>(session key)
    WS->>ClientState: clear markers list
    deactivate WS

    Conversation->>Redis: fetch markers by session key
    activate Conversation
    Conversation->>Conversation: parse JSON markers
    Conversation->>Conversation: attach to conversation.markers
    Conversation->>Conversation: save conversation
    deactivate Conversation
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title describes the main refactoring focus on device selection and user interaction, which aligns with the primary changes in connect-omi.py and the overall PR direction.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/improved-laptop-connection

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

…end Lite BLE devices

- Introduced the friend-lite-sdk, a Python SDK for OMI/Friend Lite BLE devices, enabling audio streaming, button events, and transcription functionalities.
- Added LICENSE and NOTICE files to clarify licensing and attribution.
- Created pyproject.toml for package management, specifying dependencies and project metadata.
- Developed core modules including bluetooth connection handling, button event parsing, audio decoding, and transcription capabilities.
- Implemented example usage in README.md to guide users on installation and basic functionality.
- Enhanced connect-omi.py to utilize the new SDK for improved device management and event handling.
- Updated requirements.txt to reference the new SDK for local development.

This commit lays the foundation for further enhancements and integrations with OMI devices.
- Introduced a new `markers` list in `ClientState` to collect button event data during sessions.
- Added `add_marker` method to facilitate the addition of markers to the current session.
- Implemented `on_button_event` method in the `BasePlugin` class to handle device button events, providing context data for button state and timestamps.
- Updated `PluginRouter` to route button events to the appropriate plugin handler.
- Enhanced conversation job handling to attach markers from Redis sessions, improving the tracking of button events during conversations.
@AnkushMalaker
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🤖 Fix all issues with AI agents
In `@backends/advanced/src/advanced_omi_backend/client.py`:
- Around line 54-55: The timeout path can race with websocket_controller
finalization because start_new_conversation() is scheduled via
asyncio.create_task() and close_current_conversation() is legacy and doesn't
clear markers; update start_new_conversation() to explicitly clear markers
before resetting conversation state (call self.markers.clear() or
client_state.markers.clear() as appropriate) so any leftover markers from the
previous conversation are removed even if websocket_controller finalization
hasn't run, leaving the rest of the reset (current_audio_uuid, conversation IDs,
etc.) unchanged.

In `@backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py`:
- Around line 307-316: The block reading markers uses session_key before it's
defined, causing a NameError; move or compute session_key =
f"audio:session:{session_id}" so it is defined before the
redis_client.hget(session_key, "markers") call (session_id is the identifier to
use), then assign conversation.markers from json.loads(markers_data) and save as
before; also narrow the except to catch json.JSONDecodeError and
UnicodeDecodeError (instead of broad Exception) around the json.loads/decoding
step to avoid swallowing unrelated errors.

In `@extras/friend-lite-sdk/friend_lite/bluetooth.py`:
- Around line 28-39: The connect method assigns self._client before awaiting
self._client.connect(), so if BleakClient.connect() raises the instance is left
with a non-None _client and future connect() calls are blocked; update
OmiConnection.connect to catch exceptions from await self._client.connect(), and
on any failure reset self._client to None (and optionally clear/set any related
state like self._disconnected) before re-raising the exception so the instance
can retry connecting later; look for the connect function, the _on_disconnect
callback, and usages of self._client / BleakClient to implement this change.

In `@extras/friend-lite-sdk/friend_lite/discover_characteristics.py`:
- Line 19: The module currently calls asyncio.run(main()) at import time which
triggers BLE connection; wrap that call in a standard module guard so that
main() only runs when executed as a script. Locate the top-level
asyncio.run(main()) invocation (and the async def main function) and move or
replace the bare call with a guarded call using if __name__ == "__main__": so
importing friend_lite.discover_characteristics no longer executes the BLE logic.
- Line 4: The file currently hardcodes a device MAC in the constant OMI_MAC;
remove that literal and change the script in discover_characteristics.py to
accept the device address via a CLI argument (e.g., argparse) or an environment
variable (e.g., FRIEND_DEVICE_MAC) with the CLI taking precedence, validate the
address format before use, and update any calls/usage/help text to reflect the
new parameter so no real device identifiers remain in source.

In `@extras/friend-lite-sdk/friend_lite/transcribe.py`:
- Around line 153-157: The logger.debug call uses an unnecessary f-string prefix
in the block that sends AudioStop when segment_has_audio is true; remove the
extra "f" so the call is logger.debug("Wyoming: Sending AudioStop...") in the
same conditional where AudioStop().event() is written to the client (see
references: segment_has_audio, logger.debug, client.write_event,
AudioStop().event()).
- Around line 96-100: The connection creation and connect call are outside the
try so connection failures bypass error handling and cleanup; move the
AsyncClient.from_uri(wyoming_url) and await client.connect() into the existing
try block that contains the subsequent logic (the try that currently starts
before the main processing and has the except handlers and finally cleanup),
ensuring any exceptions from AsyncClient.from_uri or client.connect are caught
by the existing except clauses and that the finally block still runs to
close/cleanup the client and call logger as appropriate.

In `@extras/local-omi-bt/connect-omi.py`:
- Around line 212-219: The async with OmiConnection(...) block never exits
because asyncio.gather(conn.wait_until_disconnected(), process_audio(),
backend_stream_wrapper()) never completes, preventing OmiConnection.__aexit__ /
disconnect() from running; fix by creating separate tasks for process_audio()
and backend_stream_wrapper() (e.g., via asyncio.create_task), await
conn.wait_until_disconnected(), then cancel and await those background tasks
before leaving the async with so that __aexit__/disconnect() runs and resources
are cleaned up; reference OmiConnection, wait_until_disconnected, process_audio,
backend_stream_wrapper, handle_ble_data, handle_button_event, __aexit__, and
disconnect when making the changes.
- Around line 74-78: The fire-and-forget call to
loop.create_task(send_button_event(state.name)) loses exceptions; change the
block to capture the created Task (e.g., task = loop.create_task(...)) and
attach a done callback that logs exceptions from task.exception() using the
existing logger so errors are not silently dropped; keep the RuntimeError except
handling for no running loop and ensure the callback checks for cancelled/None
exceptions before logging.
- Around line 215-219: Gathering wait_until_disconnected(), process_audio(), and
backend_stream_wrapper() with asyncio.gather hangs because the latter two are
infinite queue.get() loops; change to explicit task management: create tasks for
process_audio() and backend_stream_wrapper() via asyncio.create_task, await
conn.wait_until_disconnected(), then cancel the remaining tasks (task.cancel()),
and await them with return_exceptions=True while handling asyncio.CancelledError
inside those coroutines (or catching it when awaiting) to ensure clean shutdown;
update the block referencing wait_until_disconnected, process_audio, and
backend_stream_wrapper to use try/finally to perform cancellations and cleanup.

In `@extras/local-omi-bt/send_to_adv.py`:
- Around line 39-55: The send_button_event function can raise an unhandled
ConnectionClosed if _active_websocket closes after the None check; wrap the
await _active_websocket.send(...) call in a try/except that catches the
websocket connection closed error (e.g., websockets.exceptions.ConnectionClosed
or the relevant library exception) inside send_button_event, log the failure
with logger.debug/logger.info including the button_state and the exception, and
return gracefully so the exception doesn't propagate to callers.
🧹 Nitpick comments (11)
extras/friend-lite-sdk/friend_lite/discover_characteristics.py (1)

6-18: Consider adding error handling for connection failures.

BleakClient can raise BleakError, TimeoutError, or OSError if the device is unreachable. A try/except with a user-friendly message would make this diagnostic tool more robust.

extras/local-omi-bt/requirements.txt (1)

5-5: Local file reference may break in CI/Docker environments.

The file:../friend-lite-sdk reference assumes a specific relative directory layout. This works for local development but will fail in environments where the sibling directory isn't present (CI pipelines, Docker builds, or consumers who install from a different working directory). Consider publishing to a private index or using a git-based dependency as a fallback, or at minimum document this constraint clearly.

extras/friend-lite-sdk/pyproject.toml (1)

14-16: Homepage URL points to the author's fork, not the organization repo.

The Homepage URL references AnkushMalaker/chronicle rather than the target organization repository (SimpleOpenSoftware/chronicle). Since this PR targets the SimpleOpenSoftware org, consider updating the URL to reflect the canonical upstream.

-Homepage = "https://github.com/AnkushMalaker/chronicle"
+Homepage = "https://github.com/SimpleOpenSoftware/chronicle"
extras/friend-lite-sdk/friend_lite/decoder.py (1)

19-24: Use logging instead of print and narrow the exception type.

Two issues flagged by static analysis (and good practice for library code):

  1. print() in library code — Consumers cannot suppress or redirect this output. Use logging.getLogger(__name__).warning(...) instead.
  2. Catching bare Exception — This swallows programming errors (e.g., TypeError from bad arguments). Catch the specific Opus decode exception instead.
♻️ Proposed fix
+import logging
 from opuslib import Decoder
+
+logger = logging.getLogger(__name__)
 
 
 class OmiOpusDecoder:
     def __init__(self):
         self.decoder = Decoder(16000, 1)  # 16kHz mono
 
     def decode_packet(self, data: bytes, strip_header: bool = True):
         if len(data) <= 3:
             return b''
 
         # Remove 3-byte header
         if strip_header:
             clean_data = bytes(data[3:])
         else:
             clean_data = data
 
         # Decode Opus to PCM 16-bit
         try:
             pcm = self.decoder.decode(clean_data, 960, decode_fec=False)
-            return pcm
-        except Exception as e:
-            print("Opus decode error:", e)
+        except Exception:
+            logger.warning("Opus decode error", exc_info=True)
             return b''
+        return pcm
extras/friend-lite-sdk/friend_lite/transcribe.py (1)

38-83: transcribe() uses print() throughout while the module defines a logger.

This function uses print() for all output whereas transcribe_wyoming consistently uses the module-level logger. For consistency and production use (log levels, filtering, structured output), switch to logger.

extras/friend-lite-sdk/friend_lite/bluetooth.py (1)

9-12: print_devices() uses asyncio.run(), which will crash if called from within an existing event loop.

This is a synchronous convenience wrapper, but if a consumer mistakenly calls it from async context (e.g., a Jupyter notebook or an already-running loop), it will raise RuntimeError. Consider making this async or documenting the limitation.

extras/friend-lite-sdk/README.md (1)

22-30: Usage example imports ButtonState and parse_button_event but never uses them.

Consider either removing the unused imports from the example or adding a brief button-event snippet to demonstrate their use. Unused imports in documentation examples can confuse readers.

extras/friend-lite-sdk/friend_lite/button.py (1)

16-24: ButtonState(state) will raise ValueError for unrecognized state values from firmware.

If the device sends a state value not in the enum (e.g., a new firmware adds state 6), ButtonState(state) throws. This is acceptable fail-fast behavior, but worth noting since BLE data can be unpredictable. Consider whether callers should be expected to catch ValueError or if the SDK should handle it (e.g., return a raw int or a sentinel).

extras/local-omi-bt/connect-omi.py (3)

43-46: as_audio_chunks appears unused.

This function is defined but never called in the file — audio chunks are constructed inline at Line 188 instead. Consider removing it if it's no longer needed.


148-166: Probe-connect creates two connection cycles & minor lint cleanup.

The test_client variable triggers an F841 lint warning. Since you only need the side-effect of connecting, prefix it with _ to signal intent. Also, f-string formatting in logger calls eagerly evaluates even when the log level is disabled — prefer %-style.

Suggested cleanup
-            async with BleakClient(mac_address) as test_client:
-                logger.info(f"Successfully connected to device {mac_address}")
+            async with BleakClient(mac_address) as _client:
+                logger.info("Successfully connected to device %s", mac_address)
         except Exception as e:
-            logger.error(f"Failed to connect to device {mac_address}: {e}")
+            logger.error("Failed to connect to device %s: %s", mac_address, e)

Apply the same pattern to the retry block at Lines 161-166.


49-54: Consider removing the commented-out Deepgram code.

This dead code adds noise. If it's needed later, it's recoverable from version control.

Comment on lines +54 to +55
# Markers (e.g., button events) collected during the session
self.markers: List[dict] = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n -C5 'markers' --type=py -g '*websocket*'

Repository: SimpleOpenSoftware/chronicle

Length of output: 4063


🏁 Script executed:

cat -n backends/advanced/src/advanced_omi_backend/client.py | sed -n '50,75p; 105,115p; 120,170p'

Repository: SimpleOpenSoftware/chronicle

Length of output: 4243


🏁 Script executed:

rg -n 'markers' --type=py -B2 -A2 | head -100

Repository: SimpleOpenSoftware/chronicle

Length of output: 7187


Markers clearing in websocket_controller confirmed, but timeout path introduces potential race condition.

The websocket_controller does clear client_state.markers (at lines 537 and 1158) after persisting them to Redis, confirming external coordination. However, the timeout path at line 72 calls start_new_conversation() via asyncio.create_task() without blocking, meaning the websocket_controller's finalization may not have run yet when a new conversation begins, allowing markers from the previous conversation to accumulate.

Since close_current_conversation() is documented as "legacy V1 code" (lines 139–140) and doesn't participate in the websocket_controller's finalization flow, markers must be cleared explicitly in start_new_conversation() before resetting conversation state:

async def start_new_conversation(self):
    """Start a new conversation by closing current and resetting state."""
    await self.close_current_conversation()
    self.markers.clear()  # Clear before resetting conversation
    
    # Reset conversation state
    self.current_audio_uuid = None
    ...
🤖 Prompt for AI Agents
In `@backends/advanced/src/advanced_omi_backend/client.py` around lines 54 - 55,
The timeout path can race with websocket_controller finalization because
start_new_conversation() is scheduled via asyncio.create_task() and
close_current_conversation() is legacy and doesn't clear markers; update
start_new_conversation() to explicitly clear markers before resetting
conversation state (call self.markers.clear() or client_state.markers.clear() as
appropriate) so any leftover markers from the previous conversation are removed
even if websocket_controller finalization hasn't run, leaving the rest of the
reset (current_audio_uuid, conversation IDs, etc.) unchanged.

Comment on lines +307 to +316
# Attach markers from Redis session (e.g., button events captured during streaming)
markers_json = await redis_client.hget(session_key, "markers")
if markers_json:
try:
markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode()
conversation.markers = json.loads(markers_data)
await conversation.save()
logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}")
except Exception as marker_err:
logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

session_key is undefined here — runtime NameError.

session_key is first defined on line 362 (session_key = f"audio:session:{session_id}"), but this block at line 308 references it before that definition. This will crash open_conversation_job with a NameError every time markers exist in Redis.

🐛 Fix: define session_key before use
     # Attach markers from Redis session (e.g., button events captured during streaming)
+    session_key = f"audio:session:{session_id}"
     markers_json = await redis_client.hget(session_key, "markers")
     if markers_json:
         try:
             markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode()
             conversation.markers = json.loads(markers_data)
             await conversation.save()
             logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}")
-        except Exception as marker_err:
+        except (json.JSONDecodeError, UnicodeDecodeError) as marker_err:
             logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}")

Note: The except Exception is also overly broad for what should only be a JSON parsing concern — narrowing to json.JSONDecodeError and UnicodeDecodeError avoids silently swallowing unexpected failures.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Attach markers from Redis session (e.g., button events captured during streaming)
markers_json = await redis_client.hget(session_key, "markers")
if markers_json:
try:
markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode()
conversation.markers = json.loads(markers_data)
await conversation.save()
logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}")
except Exception as marker_err:
logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}")
# Attach markers from Redis session (e.g., button events captured during streaming)
session_key = f"audio:session:{session_id}"
markers_json = await redis_client.hget(session_key, "markers")
if markers_json:
try:
markers_data = markers_json if isinstance(markers_json, str) else markers_json.decode()
conversation.markers = json.loads(markers_data)
await conversation.save()
logger.info(f"📌 Attached {len(conversation.markers)} markers to conversation {conversation_id}")
except (json.JSONDecodeError, UnicodeDecodeError) as marker_err:
logger.warning(f"⚠️ Failed to parse markers from Redis: {marker_err}")
🧰 Tools
🪛 Ruff (0.14.14)

[error] 308-308: Undefined name session_key

(F821)


[warning] 315-315: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
In `@backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py`
around lines 307 - 316, The block reading markers uses session_key before it's
defined, causing a NameError; move or compute session_key =
f"audio:session:{session_id}" so it is defined before the
redis_client.hget(session_key, "markers") call (session_id is the identifier to
use), then assign conversation.markers from json.loads(markers_data) and save as
before; also narrow the except to catch json.JSONDecodeError and
UnicodeDecodeError (instead of broad Exception) around the json.loads/decoding
step to avoid swallowing unrelated errors.

Comment on lines +28 to +39
async def connect(self) -> None:
if self._client is not None:
return

def _on_disconnect(_client: BleakClient) -> None:
self._disconnected.set()

self._client = BleakClient(
self._mac_address,
disconnected_callback=_on_disconnect,
)
await self._client.connect()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

If BleakClient.connect() raises, the OmiConnection becomes permanently broken.

self._client is assigned on line 35 but if await self._client.connect() (line 39) throws, the guard on line 29 (if self._client is not None: return) will short-circuit all future connect() attempts on this instance.

🐛 Proposed fix — reset client on connection failure
     async def connect(self) -> None:
         if self._client is not None:
             return
 
         def _on_disconnect(_client: BleakClient) -> None:
             self._disconnected.set()
 
         self._client = BleakClient(
             self._mac_address,
             disconnected_callback=_on_disconnect,
         )
-        await self._client.connect()
+        try:
+            await self._client.connect()
+        except Exception:
+            self._client = None
+            raise
🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/bluetooth.py` around lines 28 - 39, The
connect method assigns self._client before awaiting self._client.connect(), so
if BleakClient.connect() raises the instance is left with a non-None _client and
future connect() calls are blocked; update OmiConnection.connect to catch
exceptions from await self._client.connect(), and on any failure reset
self._client to None (and optionally clear/set any related state like
self._disconnected) before re-raising the exception so the instance can retry
connecting later; look for the connect function, the _on_disconnect callback,
and usages of self._client / BleakClient to implement this change.

import asyncio
from bleak import BleakClient

OMI_MAC = "7F52EC55-50C9-D1B9-E8D7-19B83217C97D" # Replace with your actual MAC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded device MAC address should be removed or parameterized.

This looks like a real device identifier committed to the repository. For a utility script in an SDK package, accept the address as a CLI argument or environment variable instead of hardcoding it.

Proposed fix
-OMI_MAC = "7F52EC55-50C9-D1B9-E8D7-19B83217C97D"  # Replace with your actual MAC
+import sys
+import os
+
+OMI_MAC = os.environ.get("OMI_MAC") or (sys.argv[1] if len(sys.argv) > 1 else None)
+if not OMI_MAC:
+    print("Usage: python discover_characteristics.py <MAC_ADDRESS>")
+    print("  or set OMI_MAC environment variable")
+    sys.exit(1)
🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/discover_characteristics.py` at line 4,
The file currently hardcodes a device MAC in the constant OMI_MAC; remove that
literal and change the script in discover_characteristics.py to accept the
device address via a CLI argument (e.g., argparse) or an environment variable
(e.g., FRIEND_DEVICE_MAC) with the CLI taking precedence, validate the address
format before use, and update any calls/usage/help text to reflect the new
parameter so no real device identifiers remain in source.

if "notify" in char.properties:
print(" ✅ Notifiable (can stream data)")

asyncio.run(main())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Missing if __name__ == "__main__" guard — importing this module triggers BLE connection.

This file is inside the friend_lite package. The top-level asyncio.run(main()) will execute on any import of this module, blocking while it attempts a BLE connection. Wrap it in a main guard.

Proposed fix
-asyncio.run(main())
+if __name__ == "__main__":
+    asyncio.run(main())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
asyncio.run(main())
if __name__ == "__main__":
asyncio.run(main())
🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/discover_characteristics.py` at line 19,
The module currently calls asyncio.run(main()) at import time which triggers BLE
connection; wrap that call in a standard module guard so that main() only runs
when executed as a script. Locate the top-level asyncio.run(main()) invocation
(and the async def main function) and move or replace the bare call with a
guarded call using if __name__ == "__main__": so importing
friend_lite.discover_characteristics no longer executes the BLE logic.

Comment on lines +153 to +157
# 5. Stop Audio Segment (only if audio was sent in this window)
if segment_has_audio:
logger.debug(f"Wyoming: Sending AudioStop...")
await client.write_event(AudioStop().event())
logger.debug("Wyoming: AudioStop sent.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Extraneous f prefix on line 155.

f"Wyoming: Sending AudioStop..." has no placeholders. Remove the f prefix.

Proposed fix
-                logger.debug(f"Wyoming: Sending AudioStop...")
+                logger.debug("Wyoming: Sending AudioStop...")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# 5. Stop Audio Segment (only if audio was sent in this window)
if segment_has_audio:
logger.debug(f"Wyoming: Sending AudioStop...")
await client.write_event(AudioStop().event())
logger.debug("Wyoming: AudioStop sent.")
# 5. Stop Audio Segment (only if audio was sent in this window)
if segment_has_audio:
logger.debug("Wyoming: Sending AudioStop...")
await client.write_event(AudioStop().event())
logger.debug("Wyoming: AudioStop sent.")
🧰 Tools
🪛 Ruff (0.14.14)

[error] 155-155: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
In `@extras/friend-lite-sdk/friend_lite/transcribe.py` around lines 153 - 157, The
logger.debug call uses an unnecessary f-string prefix in the block that sends
AudioStop when segment_has_audio is true; remove the extra "f" so the call is
logger.debug("Wyoming: Sending AudioStop...") in the same conditional where
AudioStop().event() is written to the client (see references: segment_has_audio,
logger.debug, client.write_event, AudioStop().event()).

Comment on lines +74 to +78
try:
loop = asyncio.get_running_loop()
loop.create_task(send_button_event(state.name))
except RuntimeError:
logger.debug("No running event loop, cannot send button event")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fire-and-forget task loses exceptions silently.

loop.create_task() without storing the returned task means any exception raised inside send_button_event will be silently dropped (Python logs "Task exception was never retrieved" at GC time, which is unreliable). Store the task and attach an error-logging callback.

Proposed fix
             try:
                 loop = asyncio.get_running_loop()
-                loop.create_task(send_button_event(state.name))
+                task = loop.create_task(send_button_event(state.name))
+                task.add_done_callback(
+                    lambda t: t.exception() and logger.error("Button event send failed: %s", t.exception())
+                )
             except RuntimeError:
                 logger.debug("No running event loop, cannot send button event")
🧰 Tools
🪛 Ruff (0.14.14)

[warning] 76-76: Store a reference to the return value of loop.create_task

(RUF006)

🤖 Prompt for AI Agents
In `@extras/local-omi-bt/connect-omi.py` around lines 74 - 78, The fire-and-forget
call to loop.create_task(send_button_event(state.name)) loses exceptions; change
the block to capture the created Task (e.g., task = loop.create_task(...)) and
attach a done callback that logs exceptions from task.exception() using the
existing logger so errors are not silently dropped; keep the RuntimeError except
handling for no running loop and ensure the callback checks for cancelled/None
exceptions before logging.

Comment on lines +212 to +219
async with OmiConnection(mac_address) as conn:
await conn.subscribe_audio(handle_ble_data)
await conn.subscribe_button(handle_button_event)
await asyncio.gather(
conn.wait_until_disconnected(),
process_audio(),
backend_stream_wrapper(),
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

OmiConnection context manager exit is also blocked by gather.

Worth noting: since gather never completes (as described above), the async with OmiConnection block body never exits, meaning __aexit__ / disconnect() is never called. The fix proposed above (cancelling tasks after wait_until_disconnected) also resolves this — __aexit__ would then properly clean up the BLE connection.

🤖 Prompt for AI Agents
In `@extras/local-omi-bt/connect-omi.py` around lines 212 - 219, The async with
OmiConnection(...) block never exits because
asyncio.gather(conn.wait_until_disconnected(), process_audio(),
backend_stream_wrapper()) never completes, preventing OmiConnection.__aexit__ /
disconnect() from running; fix by creating separate tasks for process_audio()
and backend_stream_wrapper() (e.g., via asyncio.create_task), await
conn.wait_until_disconnected(), then cancel and await those background tasks
before leaving the async with so that __aexit__/disconnect() runs and resources
are cleaned up; reference OmiConnection, wait_until_disconnected, process_audio,
backend_stream_wrapper, handle_ble_data, handle_button_event, __aexit__, and
disconnect when making the changes.

Comment on lines +215 to +219
await asyncio.gather(
conn.wait_until_disconnected(),
process_audio(),
backend_stream_wrapper(),
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

asyncio.gather will hang after device disconnects.

wait_until_disconnected() completes when the BLE link drops, but process_audio() and backend_stream_wrapper() are infinite loops blocked on their respective queue.get() calls. Since gather waits for all tasks, the program will never exit cleanly after disconnection.

Consider cancelling the remaining tasks once disconnection is detected, e.g.:

Proposed fix using explicit task management
             async with OmiConnection(mac_address) as conn:
                 await conn.subscribe_audio(handle_ble_data)
                 await conn.subscribe_button(handle_button_event)
-                await asyncio.gather(
-                    conn.wait_until_disconnected(),
-                    process_audio(),
-                    backend_stream_wrapper(),
-                )
+                audio_task = asyncio.create_task(process_audio())
+                backend_task = asyncio.create_task(backend_stream_wrapper())
+                await conn.wait_until_disconnected()
+                logger.info("Device disconnected, shutting down…")
+                audio_task.cancel()
+                backend_task.cancel()
+                for t in (audio_task, backend_task):
+                    try:
+                        await t
+                    except asyncio.CancelledError:
+                        pass
🤖 Prompt for AI Agents
In `@extras/local-omi-bt/connect-omi.py` around lines 215 - 219, Gathering
wait_until_disconnected(), process_audio(), and backend_stream_wrapper() with
asyncio.gather hangs because the latter two are infinite queue.get() loops;
change to explicit task management: create tasks for process_audio() and
backend_stream_wrapper() via asyncio.create_task, await
conn.wait_until_disconnected(), then cancel the remaining tasks (task.cancel()),
and await them with return_exceptions=True while handling asyncio.CancelledError
inside those coroutines (or catching it when awaiting) to ensure clean shutdown;
update the block referencing wait_until_disconnected, process_audio, and
backend_stream_wrapper to use try/finally to perform cancellations and cleanup.

Comment on lines +39 to +55
async def send_button_event(button_state: str) -> None:
"""Send a button event to the backend via the active WebSocket connection.

Args:
button_state: Button state string (e.g., "SINGLE_TAP", "DOUBLE_TAP")
"""
if _active_websocket is None:
logger.debug("No active websocket, dropping button event: %s", button_state)
return

event = {
"type": "button-event",
"data": {"state": button_state},
"payload_length": None,
}
await _active_websocket.send(json.dumps(event) + "\n")
logger.info("Sent button event to backend: %s", button_state)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

send_button_event can raise unhandled ConnectionClosed if the websocket closes between the guard and the send.

Between the _active_websocket is None check on line 45 and the .send() on line 54, the connection may have been closed (e.g., server-side disconnect). This would propagate an unhandled exception to the caller.

🛡️ Proposed fix
 async def send_button_event(button_state: str) -> None:
     ...
     if _active_websocket is None:
         logger.debug("No active websocket, dropping button event: %s", button_state)
         return
 
     event = {
         "type": "button-event",
         "data": {"state": button_state},
         "payload_length": None,
     }
-    await _active_websocket.send(json.dumps(event) + "\n")
-    logger.info("Sent button event to backend: %s", button_state)
+    try:
+        await _active_websocket.send(json.dumps(event) + "\n")
+        logger.info("Sent button event to backend: %s", button_state)
+    except Exception:
+        logger.warning("Failed to send button event (connection may have closed): %s", button_state)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def send_button_event(button_state: str) -> None:
"""Send a button event to the backend via the active WebSocket connection.
Args:
button_state: Button state string (e.g., "SINGLE_TAP", "DOUBLE_TAP")
"""
if _active_websocket is None:
logger.debug("No active websocket, dropping button event: %s", button_state)
return
event = {
"type": "button-event",
"data": {"state": button_state},
"payload_length": None,
}
await _active_websocket.send(json.dumps(event) + "\n")
logger.info("Sent button event to backend: %s", button_state)
async def send_button_event(button_state: str) -> None:
"""Send a button event to the backend via the active WebSocket connection.
Args:
button_state: Button state string (e.g., "SINGLE_TAP", "DOUBLE_TAP")
"""
if _active_websocket is None:
logger.debug("No active websocket, dropping button event: %s", button_state)
return
event = {
"type": "button-event",
"data": {"state": button_state},
"payload_length": None,
}
try:
await _active_websocket.send(json.dumps(event) + "\n")
logger.info("Sent button event to backend: %s", button_state)
except Exception:
logger.warning("Failed to send button event (connection may have closed): %s", button_state)
🤖 Prompt for AI Agents
In `@extras/local-omi-bt/send_to_adv.py` around lines 39 - 55, The
send_button_event function can raise an unhandled ConnectionClosed if
_active_websocket closes after the None check; wrap the await
_active_websocket.send(...) call in a try/except that catches the websocket
connection closed error (e.g., websockets.exceptions.ConnectionClosed or the
relevant library exception) inside send_button_event, log the failure with
logger.debug/logger.info including the button_state and the exception, and
return gracefully so the exception doesn't propagate to callers.

@github-actions
Copy link

github-actions bot commented Feb 8, 2026

⚠️ Robot Framework Test Results (No API Keys)

Status: ❌ Some tests failed

ℹ️ Note: This run excludes tests requiring external API keys (Deepgram, OpenAI).
Tests tagged with requires-api-keys will run on dev/main branches.

Metric Count
✅ Passed 102
❌ Failed 17
📊 Total 119

📊 View Reports

GitHub Pages (Live Reports):

Download Artifacts:


View full workflow run

@AnkushMalaker AnkushMalaker deleted the feat/improved-laptop-connection branch February 22, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant